Skip to content

Fix for zones reopening in simulation-based constraint mechanism for work/school location choice#1028

Merged
jpn-- merged 5 commits intoActivitySim:mainfrom
RSGInc:shadow_pricing_reopening
Jan 15, 2026
Merged

Fix for zones reopening in simulation-based constraint mechanism for work/school location choice#1028
jpn-- merged 5 commits intoActivitySim:mainfrom
RSGInc:shadow_pricing_reopening

Conversation

@dhensle
Copy link
Contributor

@dhensle dhensle commented Dec 30, 2025

Fix for #820: If zones close, they now remain closed.

@jpn-- jpn-- self-requested a review January 6, 2026 19:15
Copy link
Member

@jpn-- jpn-- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Here's why I like it:

  • You have written a clear and simple test that creates a tiny model, runs it, and fails on the existing code in a manner that clearly demonstrates the existing problem and runs in under one second.
  • After the code changes are made, the test passes. Voila!

The one thing that's missing is a note on the change log activitysim/docs/dev-guide/changes.md. This is not really what may of us would call a "breaking change" as it should not formally cause any existing model to fail, but it does break strict reproducibility between versions. This is seen in the SANDAG test, which now fails because the shadow-priced location choices are changing just a tiny bit.

@jpn-- jpn-- added this to Phase 11 Jan 8, 2026
@jpn-- jpn-- moved this to Under Review in Phase 11 Jan 8, 2026
@dhensle
Copy link
Contributor Author

dhensle commented Jan 8, 2026

Great! Glad you like the test -- it was not a trivial exercise to get a nice CI test built for this particular issue. And we also now have a CI test for the simulation-based constraint mechanism as a whole as a bonus!

I have updated the change log. I think this is ready to merge!

@jpn-- jpn-- merged commit aa58cc4 into ActivitySim:main Jan 15, 2026
17 of 19 checks passed
@github-project-automation github-project-automation bot moved this from Under Review to Done in Phase 11 Jan 15, 2026
dhensle added a commit to RSGInc/activitysim that referenced this pull request Mar 4, 2026
commit c031c8b
Author: David Hensle <hensle93@gmail.com>
Date:   Fri Feb 13 15:48:35 2026 -0800

    adding ignore list to skim load for unused skims

commit 83613b2
Author: Copilot <198982749+Copilot@users.noreply.github.com>
Date:   Thu Feb 12 16:30:58 2026 -0600

    Fix Sphinx currentmodule directive for location choice documentation (ActivitySim#1034)

    * Initial plan

    * Fix currentmodule directive in work_location_choice.md and school_location_choice.md

    Co-authored-by: jpn-- <1036626+jpn--@users.noreply.github.com>

    ---------

    Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
    Co-authored-by: jpn-- <1036626+jpn--@users.noreply.github.com>

commit aa58cc4
Author: David Hensle <51132108+dhensle@users.noreply.github.com>
Date:   Fri Jan 16 03:31:07 2026 +1100

    Fix for zones reopening in simulation-based constraint mechanism for work/school location choice (ActivitySim#1028)

    * addresses issue with zones reopening after being closed in shadow_pricing

    * addresses issue with zones reopening after being closed in shadow_pricing

    * adding shadow price regression values

    * updating change log

    ---------

    Co-authored-by: juangacosta <juan.acosta@rsginc.com>

commit 6266ce4
Author: David Hensle <51132108+dhensle@users.noreply.github.com>
Date:   Thu Jan 8 10:26:48 2026 -0800

    Index names consistently set to "alt" for alternatives files (ActivitySim#1018)

    * "alt" now used as index in all alternatives modules

    * formatting

    * minor commenting

    * using stable sorting for unit test

    * adding to change log

    * updating school escort bundle Alt to alt

    * typo

    * Revert "updating school escort bundle Alt to alt"

    This reverts commit 625f685.

    * regenerate pipeline

    ---------

    Co-authored-by: juangacosta <juan.acosta@rsginc.com>
    Co-authored-by: Jeffrey Newman <jeff@driftless.xyz>

commit 0811191
Author: Jeffrey Newman <jeff@driftless.xyz>
Date:   Thu Jan 8 11:11:43 2026 -0600

    Repair CDAP estimation (ActivitySim#1026)

    * fix error in CDAP estimation model construction

    * update tests for CDAP

    * fix the other test file

    * add to changes.md

    * note on possible fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants